fix(iOS): unregister tabs accessory observer from observed wrapper#3948
Open
safaiyeh wants to merge 2 commits intosoftware-mansion:mainfrom
Open
fix(iOS): unregister tabs accessory observer from observed wrapper#3948safaiyeh wants to merge 2 commits intosoftware-mansion:mainfrom
safaiyeh wants to merge 2 commits intosoftware-mansion:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a potential iOS 26 crash during bottom accessory invalidation by ensuring KVO is unregistered from the exact wrapper view instance that was originally observed (rather than re-deriving the wrapper via the current superview chain at teardown time).
Changes:
- Track and store the specific native wrapper view used for KVO registration, and unregister from that stored instance during invalidation.
- Make frame-change KVO registration idempotent for the same wrapper and safely re-register if the wrapper view changes.
- Use a dedicated KVO context and forward unrelated observations to
super.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kligarski
reviewed
Apr 30, 2026
| @implementation RNSTabsBottomAccessoryHelper { | ||
| RNSTabsBottomAccessoryComponentView *__weak _bottomAccessoryView; | ||
| UIView *__weak _observedNativeWrapperView; | ||
| BOOL _isObservingNativeWrapperView; |
Contributor
There was a problem hiding this comment.
It seems to me that this boolean is redundant. If _observedNativeWrapperView is nil, we know that we don't observe anything. It also introduces possibility of inconsistent state (e.g. _isObservingNativeWrapperView = YES and _observedNativeWrapperView = nil) so I don't think we should keep it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes a possible iOS 26 bottom accessory crash during invalidation.
The helper registers KVO for
centeronself.nativeWrapperView, but invalidation removed the observer from_bottomAccessoryView.superview.superviewat teardown time. UIKit can detach or swap the bottom accessory wrapper before invalidation runs, so the teardown path may attempt to remove the observer from a different wrapper view and raise anNSRangeExceptionbecause that object was never observed.I do not see an existing issue for this exact stack, but we observed it in production on
react-native-screens4.23.0, and the same registration/removal pattern is present on currentmain.Changes
registerForAccessoryFrameChangesidempotent for the same wrapper and unregisters the previous wrapper if the wrapper changes.super.Test plan
yarn install --immutableyarn prepareyarn check-typesxcrun clang-format -i ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.mmgit diff --checkExisting examples that exercise bottom accessory behavior:
apps/src/tests/issue-tests/Test3288.tsxapps/src/tests/single-feature-tests/tabs/bottom-accessory-layout.tsxI did not add a JS unit test for this because the failure is caused by UIKit/KVO object identity during native view invalidation. A meaningful automated regression test would need a native iOS XCTest harness or an iOS integration/e2e test that drives bottom accessory mount, detach, and invalidation lifecycle. This repository does not appear to have an iOS native unit-test target for library internals today.
I attempted
yarn test:unitafter initializing thereact-navigationsubmodule, but the root Jest command also collects the submodule's own test suites and fails on unrelated submodule Jest setup / React version issues before exercising this native change.Checklist